-
-
Notifications
You must be signed in to change notification settings - Fork 475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(config): handle unset
and off
values in editorconfig files
#3907
fix(config): handle unset
and off
values in editorconfig files
#3907
Conversation
c223874
to
f0e2039
Compare
crates/biome_cli/tests/snapshots/main_cases_editorconfig/should_emit_diagnostics.snap
Outdated
Show resolved
Hide resolved
```block | ||
configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
||
× Key 'max_line_length' is incompatible with biome: 'off' is incompatible with how Biome works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec says that off
means "use the default settings". Can't we do that?
Regarding the message, I think we could be more specific. Saying "incompatible with Biome" seems to be too generic. Also, the message isn't actionable, because it doesn't tell the user what they should do to fix the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we can do that.
Btw, the only mention of "off" seems to be in this document: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties and it doesn't seem to be present in https://spec.editorconfig.org/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I noticed that too 🥲
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps they standardized off
to unset
?
f0e2039
to
ca791ca
Compare
ca791ca
to
045f3da
Compare
Summary
In the editorconfig spec,
unset
means "use the default value" andoff
(in the context ofmax_line_length
) means "completely turn off the line limit".This PR aims to implement the behavior for
unset
, andoff
.provide a better diagnostic when encounteringoff
. We technically can't really handleoff
because biome doesn't have a mechanism to turn off the max line length enforcement.fixes #3886
closes #3904
Test Plan
Added some unit tests and a cli snapshot test